Skip to content

Fix fixed-hash#110

Merged
bkchr merged 3 commits into
paritytech:masterfrom
koushiro:fix/fixed-hash-const
Mar 7, 2019
Merged

Fix fixed-hash#110
bkchr merged 3 commits into
paritytech:masterfrom
koushiro:fix/fixed-hash-const

Conversation

@koushiro
Copy link
Copy Markdown
Contributor

@koushiro koushiro commented Feb 21, 2019

Signed-off-by: koushiro koushiro.cqx@gmail.com

Fix the problem that the current version of fixed-hash cannot be defined in constant directly.

related issue link: #109

…tant directly

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@parity-cla-bot
Copy link
Copy Markdown

It looks like @koushiro hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

@koushiro
Copy link
Copy Markdown
Contributor Author

[clabot:check]

@parity-cla-bot
Copy link
Copy Markdown

It looks like @koushiro signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
Copy link
Copy Markdown
Contributor

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey,

Technically this is added functionality but it should be backwards compatible right?

Lastly, please respect the .editorconfig and we are not using cargo fmt please revert that commit

EDIT:
Well const fns doesn't look to be stable anytime soon so I guess that this make sense. We should probably add a tracking issue for that

However, if we merge this and switch back it is a breaking change just saying.

This reverts commit 529720d.

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
Copy link
Copy Markdown

@Robbepop Robbepop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like small PRs. Looks good to me and might help as long as there is no stable support for const-fn.

Copy link
Copy Markdown

@larsrh larsrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much in favour of this. Without this change, the storage_keys! macro breaks.

@Robbepop
Copy link
Copy Markdown

Robbepop commented Mar 7, 2019

Very much in favour of this. Without this change, the storage_keys! macro breaks.

Glad that you pointed this out!
However, on the other side the fix for storage_keys seems to be simple.

@larsrh
Copy link
Copy Markdown

larsrh commented Mar 7, 2019

The way I'm working around this right now looks as follows:

pub struct Key {
    pub key: [u8; 32]
}

impl Key {
    pub fn new(key: [u8; 32]) -> Key {
        Key { key: key }
    }

    pub fn h256(&self) -> H256 {
        H256::from(self.key)
    }
}

#[macro_export]
macro_rules! storage_keys {
	() => {};
	($($name:ident),*) => {
		storage_keys!(0u8, $($name),*);
	};
	($count:expr, $name:ident) => {
		static $name: Key = Key { key: [$count, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0] };
	};
	($count:expr, $name:ident, $($tail:ident),*) => {
		static $name: Key = Key { key: [$count, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0] };
		storage_keys!($count + 1u8, $($tail),*);
	};
}

Essentially, I'm building my own key type for storage.

@Robbepop
Copy link
Copy Markdown

Robbepop commented Mar 7, 2019

I ll merge it as soon as CI is okay with it.
The current CI reports an issue with Windows but it doesn't look right so I just restarted it.

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Mar 7, 2019

@Robbepop travis windows is always broken and after they kicked out all people, it will probably never work.

@bkchr bkchr merged commit c64e1b5 into paritytech:master Mar 7, 2019
ordian added a commit that referenced this pull request Mar 27, 2019
* master:
  Fix fixed-hash (#110)
  Update transaction-pool/src/error.rs
  tx-pool: I think I copied and pasted some spaces from somewhere
  tx-pool: remove more spaces
  tx-pool: where did those spaces come from
  tx-pool: make error generic over hash
  tx-pool: replace error-chain with vanilla error impl
@koushiro koushiro deleted the fix/fixed-hash-const branch May 27, 2019 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants